-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: delete model #134
feat: delete model #134
Conversation
8e3d2ce
to
408a6d8
Compare
this.#deleted.add(modelId); | ||
await this.sendModelsInfo(); | ||
try { | ||
await fs.promises.rm(modelDir, { recursive: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recursive option for deleting always scares me, something we could do, is listing all the files inside the model directory, ask user for confirmation something like
"The following files will be deleted
- model-a.gguff
"
Then delete them by name instead of deleting the folder ? (I am maybe just overreacting)
|
||
<ListItemButtonIcon | ||
icon={faTrash} | ||
onClick={() => deleteModel()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we need confirmation modal
ce3c39c
to
425ef27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes |
@feloy maybe i got what happened. I had my playground running and i tried to delete the model 🤔 I'll try again, but we should inform the user in this case. UPDATE: I confirm. It works fine when the model is not used. |
Yes, I would like to display the error message to the user in this case, but for now, AI Studio doesn't have a clear way to display errors coming from the backend (as the call is async, we cannot get the error as the result from a call to a @jeffmaury what do you think? Do we want to implement a generic error handler (kind of notifications), something else? |
I saw Axel just pushed #152 . Maybe adding a task to the taksmanager and an error icon next to the delete icon that opens up the task manager? |
91d14a9
to
72db6d8
Compare
72db6d8
to
9861dc9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried again, now the error is displayed on a dialog. Thanks!
9861dc9
to
1aa5701
Compare
fixes partially #50